London | 26-ITP-Jan | Angela McLeary | Sprint 2 | Coursework/sprint 2#1110
London | 26-ITP-Jan | Angela McLeary | Sprint 2 | Coursework/sprint 2#1110AngelaMcLeary wants to merge 20 commits intoCodeYourFuture:mainfrom
Conversation
This comment has been minimized.
This comment has been minimized.
AngelaMcLeary
added
the
Needs Review
This comment has been minimized.
This comment has been minimized.
github-actions
bot
removed
the
Needs Review
AngelaMcLeary
changed the title
AngelaMcLeary
added
the
Needs Review
tee4tao
added
Review in progress
Sprint-2/1-key-errors/0.js
Outdated
|
|
||
| function capitalise(str) { | ||
| let str = `${str[0].toUpperCase()}${str.slice(1)}`; | ||
| str = `${str[0].toUpperCase()}${str.slice(1)}`; |
There was a problem hiding this comment.
In small examples this work, but do you think modifying function parameters is a good practice in larger codebases?
Could you rewrite this function in a way that avoids mutating the parameter entirely?
There was a problem hiding this comment.
@tee4tao thank you for your feedback.
I have updated the code to return the value without declaring str.
Thank you,
Angela.
Sprint-2/1-key-errors/1.js
Outdated
| return `${decimalNumber * 100}%`; | ||
| } | ||
|
|
||
| console.log(decimalNumber); No newline at end of file |
There was a problem hiding this comment.
Hi @tee4tao thank you for your feedback.
That was an error, it should have been console.log(convertToPercentage(0.5));. I have corrected this.
Thank you
Angela,
| //the computer reads the values in the second console.log "multiply(10, 32)" and | ||
| //the first console.log only prints the value, it does not store it. | ||
| //the second console.log outputs the template literals and undefined | ||
| // as it has no value to reach. |
There was a problem hiding this comment.
In your explanation, you mentioned that the second console.log outputs undefined 'as it has no value to reach.' You are exactly right! The technical term for this in JavaScript is an implicit return. If you don't explicitly tell a function what to return, JavaScript automatically forces it to return undefined behind the scenes. Keep up the great work!
There was a problem hiding this comment.
@tee4tao Thank you for the feedback. I will note that for future explanations.
Thank you
Angela.
| // So every console.log will print 3 every time. This function is not | ||
| // re-useable as it stands. |
There was a problem hiding this comment.
Good point and observation. This function is not re-useable as it stands, functions should ideally be self-contained and reusable.
| function calculateBMI(weight, height) { | ||
| // return the BMI of someone based off their weight and height | ||
| } No newline at end of file | ||
| return (weight/(Math.pow(height, 2))).toFixed(1); |
There was a problem hiding this comment.
Using .toFixed() converts your datatype to String. How might you convert that final answer back into a true Number before returning it?
You used Math.pow(height, 2) to square the height, which is 100% correct. However, modern JavaScript has a shorthand operator for exponents that makes equations look a bit cleaner.
There was a problem hiding this comment.
@tee4tao , Thanks for the questions and feedback.
1. I have use JavaScript's inbuilt Number function to convert the string to a number: return Number((weight / (height ** 2)).toFixed(1)); This should fix the problem of returning a string from using .toFixed.
2. The other way to square the height with modern JavaScript by using "**" instead of Math.pow.
(weight / (height ** 2); This should bring thr code uptodate with modern styles. I have also updated the code accordingly.
Thank you
Angela,
tee4tao
added
Reviewed
…used modern js to square the height
…t output correctly
2 participants
Learners, PR Template
Self checklist
Changelist
It is about debugging and understanding how logic works.